Fixes #36849 - Run GHA on Ruby 3.0#9989
Conversation
fde7cd8 to
2a7cda3
Compare
ekohl
left a comment
There was a problem hiding this comment.
I had a quick look at this and it appears the approach is to make the constructor's signature match the base class. That makes sense to me.
As for STI, I'm not sure we can get rid of it. I'd like to. At this moment I think we only have 2 implementations, so be sure to check out what Discovery does.
2a7cda3 to
c6dd66a
Compare
c6dd66a to
d49fd7e
Compare
| { | ||
| "postgresql": ["12"], | ||
| "ruby": ["2.7"], | ||
| "ruby": ["2.7", "3.0"], |
There was a problem hiding this comment.
Can we separate this out to its own commit? I'd like to merge all the preparation work before we pull the trigger.
ekohl
left a comment
There was a problem hiding this comment.
There are some deprecation warnings when running tests:
2024-01-23T18:54:00.9931233Z Mocha deprecation warning at /home/runner/work/foreman/foreman/app/services/proxy_api/dhcp.rb:36:in `unused_ip': Expectation defined at /home/runner/work/foreman/foreman/test/unit/proxy_api/dhcp_test.rb:99:in `block in <class:ProxyApiDhcpTest>' expected positional hash ({:query => "mac=00:11:22:33:44:55"}), but received keyword arguments (:query => "mac=00:11:22:33:44:55"). These will stop matching when strict keyword argument matching is enabled. See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.
2024-01-23T18:54:01.0281310Z Mocha deprecation warning at /home/runner/work/foreman/foreman/app/services/proxy_api/dhcp.rb:36:in `unused_ip': Expectation defined at /home/runner/work/foreman/foreman/test/unit/proxy_api/dhcp_test.rb:74:in `block in <class:ProxyApiDhcpTest>' expected positional hash ({:query => ""}), but received keyword arguments (:query => ""). These will stop matching when strict keyword argument matching is enabled. See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.
2024-01-23T18:54:01.0407752Z Mocha deprecation warning at /home/runner/work/foreman/foreman/app/services/proxy_api/dhcp.rb:36:in `unused_ip': Expectation defined at /home/runner/work/foreman/foreman/test/unit/proxy_api/dhcp_test.rb:90:in `block in <class:ProxyApiDhcpTest>' expected positional hash ({:query => "from=192.168.0.50&to=192.168.0.150"}), but received keyword arguments (:query => "from=192.168.0.50&to=192.168.0.150"). These will stop matching when strict keyword argument matching is enabled. See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.
2024-01-23T18:54:13.7290895Z /home/runner/work/foreman/foreman/vendor/bundle/ruby/2.7.0/gems/fog-vsphere-3.6.3/lib/fog/vsphere/compute.rb:344: warning: deprecated Object#=~ is called on Integer; it always returns nil
2024-01-23T18:54:13.7458370Z /home/runner/work/foreman/foreman/vendor/bundle/ruby/2.7.0/gems/fog-vsphere-3.6.3/lib/fog/vsphere/compute.rb:344: warning: deprecated Object#=~ is called on Integer; it always returns nil
2024-01-23T18:54:22.8953091Z Mocha deprecation warning at /home/runner/work/foreman/foreman/app/models/host/managed.rb:715:in `ipmi_boot': Expectation defined at /home/runner/work/foreman/foreman/test/models/host_test.rb:3245:in `block (2 levels) in <class:HostTest>' expected keyword arguments (:function => "bootdevice", :device => "bios"), but received positional hash ({:function => "bootdevice", :device => "bios"}). These will stop matching when strict keyword argument matching is enabled. See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.
The fog-vsphere warnings will break with Ruby 3.2. The other ones look like some things we should fix in our code and then enable strict_keyword_argument_matching in mocha.
d49fd7e to
0aab057
Compare
|
Can we also enable strict keyword argument matching (https://mocha.jamesmead.org/Mocha/Configuration.html#strict_keyword_argument_matching=-instance_method)? Perhaps make that the whole second commit? |
ecb8a73 to
047dceb
Compare
|
Done. Not sure though if we need that. As there was stated, this will be |
047dceb to
b17c9c8
Compare
That way we don't accidentally regress now that we fixed the offenders until it's |
b17c9c8 to
eefdccd
Compare
|
Indeed. If it was |
|
Some Katello test failures are related to strict keyword argument matching I guess, but otherwise 🍏 for Foreman. |
I've love to leverage what you did in #9989 (comment) and see how other repositories behave. |
I'm preparing more PRs regarding what I found in Katello, I'll paste them here once they are ready. The plan to go over my locally checkedout other plugins as well. |
@ekohl, there is another, more general and detailed approach trying to fix the same issue. I hope the code and the comments will help to understand the idea of what's going wrong.
Unfortunately, that's the last thing I could find. The third and (that's only my opinion) the last option would be to completely get rid of that weird
app/models/host.rbmodule, which (per my understanding) simply tries to simulate an abstract class.UPD: TIL that rubocop can apparently yell that the indentation of a comment can be wrong and that's unacceptable 🤦
Alternative to #9871.